Migrate bin bindings to new BindingGenerator trait#2880
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the binary bindings code to use the BindingGenerator trait pattern, consolidating wheel writing logic and improving code organization. The key architectural change is deferring .dist-info writing in WheelWriter until finish() to allow generators to mutate metadata during artifact processing, which aligns with PEP 427 recommendations.
Key Changes
- Introduced
BinBindingGeneratorimplementing theBindingGeneratortrait to handle binary artifacts, consolidating logic from the removedbin_wasi_helperfunction andwrite_binfunction - Modified
WheelWriterto defer writing.dist-infodirectory untilfinish(), movingpyproject_dirandtagsparameters from constructor tofinish()method - Updated
BindingGeneratortrait to use&mut selfmethods, allowing generators to maintain mutable state across artifact processing
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/module_writer/wheel_writer.rs | Refactored to defer .dist-info writing from constructor to finish() method; updated test accordingly |
| src/build_context.rs | Removed bin_wasi_helper function; updated all wheel writing methods to use new WheelWriter API; converted add_external_libs and add_rpath to use generic Borrow parameters for flexibility |
| src/binding_generator/wasm_binding.rs | Deleted file - functionality migrated to bin_binding.rs |
| src/binding_generator/pyo3_binding.rs | Updated generate_bindings method signature to &mut self |
| src/binding_generator/cffi_binding.rs | Updated generate_bindings method signature to &mut self |
| src/binding_generator/bin_binding.rs | New file implementing BinBindingGenerator with logic migrated from removed functions; handles binary artifact packaging and WASI launcher generation |
| src/binding_generator/mod.rs | Updated BindingGenerator trait to use &mut self; modified generate_binding to accept slice of artifacts with generic Borrow parameter; removed write_bin function |
e-nomem
force-pushed
the
bin-bindings
branch
from
1924d57 to
30719a5
Compare
messense
approved these changes
Dec 1, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updating the generator code required a few changes:
BindingGeneratortrait methods are now&mut selffn generate_bindings(..)to take&[&BuildArtifact]and loop over them now to handle multiple bin filesWheelWriternow defers writing the.dist-infountilfinish()For that third change, there are two reasons for it:
BinBindingGeneratormutates theMetadata24as each bin is processed, so we cannot write theentry_points.txtfile until the end.dist-infofiles be placed physically at the end of the archive (link)